Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/screenshare with audio #346

Closed

Conversation

kakxem
Copy link

@kakxem kakxem commented Dec 15, 2022

No description provided.

@kakxem
Copy link
Author

kakxem commented Dec 15, 2022

I resolved the conflicts, but I can't try the changes until this weekend.

@SpacingBat3 SpacingBat3 linked an issue Dec 17, 2022 that may be closed by this pull request
@orowith2os
Copy link

Builds are failing:


Error: sources/code/common/main.ts(127,8): error TS2307: Cannot find module 'node-pipewire' or its corresponding type declarations.
Error: sources/code/main/windows/main.ts(29,35): error TS2307: Cannot find module 'node-pipewire/build/types' or its corresponding type declarations.
Error: sources/code/main/windows/main.ts(49,10): error TS2307: Cannot find module 'node-pipewire' or its corresponding type declarations.
Error: sources/code/main/windows/main.ts(56,54): error TS7006: Parameter 'node' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(59,38): error TS7006: Parameter 'node' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(576,18): error TS2307: Cannot find module 'node-pipewire' or its corresponding type declarations.
Error: sources/code/main/windows/main.ts(579,69): error TS7006: Parameter 'node' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(580,67): error TS7006: Parameter 'node' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(583,71): error TS7006: Parameter 'node' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(583,77): error TS7006: Parameter 'index' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(616,22): error TS2307: Cannot find module 'node-pipewire' or its corresponding type declarations.
Error: sources/code/main/windows/main.ts(626,71): error TS7006: Parameter 'port' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(628,47): error TS7006: Parameter 'link' implicitly has an 'any' type.
Error: sources/code/main/windows/main.ts(640,65): error TS7006: Parameter 'node' implicitly has an 'any' type.
Error: Final attempt failed. Child_process exited with error code 1

@PhantomShift
Copy link

PhantomShift commented Mar 25, 2023

Using this now, one question. There's no window share available? I appear to only be able to select entire screens and not windows.

@SpookySkeletons This appears to be an issue with the latest version of electron (not sure if this is a known/reported bug?) as downgrading from major version 22 to 21 solved the issue you mentioned for me, giving me the choice to choose either a screen or a window.

Otherwise I can say that audio sharing seems to be working fine on both x11 and wayland (Arch, KDE Plasma), even streamed gameplay for a couple of hours when I had initially intended to just test to see if the feature was working.

@SpookySkeletons
Copy link

SpookySkeletons commented Apr 13, 2023

@PhantomShift Any word on latest electron 24?
Webcord just updated latest 4.2.0 build to it.

EDIT: I am unable to downgrade as electron sub 23 suffers a bug that crashes the renderer during long call sessions.

@beh-10257
Copy link

@SpookySkeletons I just made this issue in the brave browser
brave/brave-browser#29710
since well I guess they can at least post an issue in chromium issue tracker so yeah that's cool

  • there is also already a bug report made by me as well here broken screenshare #402
  • do you guys have any temporary solutions to this issue

@SpacingBat3 SpacingBat3 removed the status:wontfix This will not be worked on label Jul 19, 2023
@SpookySkeletons
Copy link

Hello, may we get a rebase of this branch to help anyone wanting to still roll these patches?

@kakxem
Copy link
Author

kakxem commented Nov 21, 2023

@SpacingBat3, the last pipeline says something about 'enums' in lines 391, 394, 395...

I'm checking the code and I found the enums you've used for representing different favicon states could be causing the error. Could you please check this particular part of your code?

Thanks!

@SpacingBat3
Copy link
Owner

Could you please check this particular part of your code?

If that's since the new TypeScript - will do once I'll move myself to it, right now I'm nowhere near my main dev machine and I might lack of the time...

@kakxem kakxem force-pushed the feature/screenshare-with-audio branch from aa35696 to 00ecf70 Compare January 21, 2024 03:03
@ThatOneCalculator
Copy link

Is this relevant anymore, given #154 ?

@SpacingBat3
Copy link
Owner

For upstream, not really, but this PR still inspires me to make things modular in the future and to allow people replacing some Electron stuff with their own native modules for better integration. I kinda feel bad about entirely killing this PR.

I should also say, both this PR's and Electron's implementations have their advantages and disadvantages, I think with this one you have a greater control over the audio within the application, yet again Electron's implementation is much more portable and doesn't rely on PipeWire at all (for some this is a bad thing, but given there are still distros that are PulseAudio only, I still think it's more portable to implement things based on Pulse than PipeWire).

@kakxem
Copy link
Author

kakxem commented Feb 22, 2024

My primary goal was to enable screen sharing with sound directly, without relying on external programs or scripts. If the proposed solution had been implemented, It's a win!

It's a bit sad that the PR was closed but I understand the decision of choosing the native solution. Although it's a bit limited in what you can do, I think it'll be easier to maintain.

Despite the outcome, I'm satisfied with the knowledge and skills I've acquired during this process.

@beh-10257
Copy link

@kakxem I have used this pr for a very long time
And well all I can say is it works beautifully with qpwgraph
It gives me all the power and tricks I need

Hope the native solution is in the same boat
Thanks for your hard work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sound in screen share on Linux